Skip to content

Use ASN.1 DER instead of MessagePack#111

Open
fjarri wants to merge 2 commits intonucypher:masterfrom
fjarri:asn1-der
Open

Use ASN.1 DER instead of MessagePack#111
fjarri wants to merge 2 commits intonucypher:masterfrom
fjarri:asn1-der

Conversation

@fjarri
Copy link
Copy Markdown
Contributor

@fjarri fjarri commented Jan 15, 2023 •
edited
Loading

This PR is supposed to replace MessagePack with ASN.1 DER as the preferred binary format.

There are several problems though:

  • serde_asn1_der is semi-abandoned and reports the format as human-readable, causing storage inefficiencies.
  • picky-asn1-der, a fork of serde_asn1_der, which is used in this PR, does not support Optional fields (or rather, it does not serialize None, so a subsequent deserialization fails - let's wait for the resolution on Support for Optional in ASN.1 Devolutions/picky-rs#201)
  • Also picky-asn1-der does not support mappings, which is not critical for this crate, but nucypher-core has those in its data structures.

Since the whole point of this change is to use a "standard" format, we should establish whether the lack of support for Optional and mappings is a limitation of the standard, or just a lack of a feature in picky-asn1-der. In the latter case we can make a PR to it, adding that support. Evidently, mappings do occur in formats that use ASN.1, e.g. VarBind here encoded as a sequence of 2-tuples.

@fjarri fjarri added the ABI Changes the format of serialized objects label Jan 15, 2023
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jan 15, 2023 •
edited
Loading

Codecov Report

Merging #111 (aaeb755) into master (5080d6d) will not change coverage.
The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master     #111   +/-   ##
=======================================
  Coverage   48.44%   48.44%           
=======================================
  Files          17       17           
  Lines         997      997           
=======================================
  Hits          483      483           
  Misses        514      514
Impacted Files Coverage Δ
umbral-pre/src/capsule_frag.rs 79.68% <ø> (ø)
umbral-pre/src/key_frag.rs 84.94% <ø> (ø)
umbral-pre/src/serde_bytes.rs 41.86% <ø> (ø)
umbral-pre/src/traits.rs 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@CBenoit CBenoit mentioned this pull request Mar 6, 2023
Merged
@KizzyCode
Copy link
Copy Markdown

KizzyCode commented Mar 6, 2023

Just as a side note: serde_asn1_der is not really abandoned – currently I'm working on a v1 release of the underlying asn1_der-crate (which however may take some time since I don't really want to soften it's no-panic approach and subsequent "guarantees").

Since serde_asn1_der serves as a simple interface to asn1_der, it cannot really progress independently (apart from bug-fixes obviously). However, I'm always open for feature-requests or PRs, especially if I missed a useful feature that can be implemented on top of the current asn1_der crate version 😊

Since DER's type system is not fully compatible to serde's type system, there is probably still a lot of room for improvements I have missed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

ABI Changes the format of serialized objects

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fjarri @codecov-commenter @KizzyCode